-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make use of Base.donotdelete if available #275
Conversation
I am not sure that running this unconditionally is the best idea. One of the stated intentions for BenchmarkTools has always been that it will execute the JuliaCode as is, to match the actual behavior as close as possible. I see the utility for BaseBenchmarks, but maybe it should change the definition to include |
I would think that most benchmarks don't actually have any side-effects, so as the compiler gets better I think we'll be deleting them more and more. I think people would expect that the return value from their benchmarks would get preserved (as it would if you just run it at the REPL, because then it gets assigned to |
A better intention seems to me to have BenchmarkTools be as useful as possible. And when it optimizes away the whole benchmark, it is not being useful. The hoops you have to jump through now with interpolating |
Base is ready to merge, so let's get this in, so we can do the Base nanosoldier run. |
After this PR julia> using BenchmarkTools; @btime 1 + 2
1.413 ns (0 allocations: 0 bytes)
julia> |
Ah, sorry. An earlier version of the built-in returned it's result. I'm not at a computer right now, but just put in a let statement or something that properly returns the result. |
You mean like this: --- a/src/execution.jl
+++ b/src/execution.jl
@@ -480,7 +480,10 @@ function generate_benchmark_definition(eval_module, out_vars, setup_vars, quote_
core_body = :($(core); $(returns))
end
if isdefined(Base, :donotdelete)
- invocation = :(Base.donotdelete($invocation))
+ invocation = :(let x = $invocation
+ Base.donotdelete(x)
+ x
+ end)
end
return Core.eval(eval_module, quote
@noinline $(signature_def) = begin $(core_body) end ? I'm not entirely sure whether |
Yes, that patch would work |
Goes with JuliaLang/julia#44036.